Adding fido appid extension #229

leshi · 2016-10-03T01:33:27Z

Closes Issue #216

rlin1 · 2016-10-04T06:43:09Z

Please rename extension to webauthn_appid as it is used by other FIDO spec families as well.

vijaybh · 2016-10-04T06:51:44Z

index.bs

+:: A single UTF-8 encoded string specifying a FIDO U2F |appId|.
+
+: Client processing
+:: AppId validation (as defined by the FIDO U2F specification).  If valid, the


Need a real reference for this. Please contribute to http://www.specref.org/ or add to the biblio block at the end of the Bikeshed source (it's also in SpecRef format).

Added a PR in specref: tobie/specref#298. Feel free to upvote.

Merged: http://www.specref.org/?q=FIDO-APPID

leshi · 2016-10-05T04:01:05Z

@rlin1, thanks for the suggestion! Done.

Closes Issue #216

leshi · 2016-10-05T21:54:37Z

@vijaybh, any idea how to update travic cl's bikeshed to pull in the new biblio data?

tobie · 2016-10-05T21:57:02Z

/cc @tabatkins (--sorry, thought this was about the api on csswg.org.)

plinss · 2016-10-05T22:08:06Z

You should run 'bikeshed update' after install to get the latest spec/biblio data. Alternatively you can use the online bikeshed at https://api.csswg.org/bikeshed (which is always up to date)

jcjones · 2016-10-05T23:03:42Z

@leshi: Can you do an update of the bikeshed data in the repo (in .spec-data) as another commit on this PR? That should fix it.

It'd be something like:

git clone --depth=100 --branch=master https://github.com/tabatkins/bikeshed.git ./bikeshed
pip install --editable ./bikeshed
bikeshed update
cp -r ./bikeshed/bikeshed/spec-data/* .spec-data/

and then test, and commit the changeset.

leshi · 2016-10-05T23:32:13Z

Thanks, @jcjones, that's what I was looking for!

Thanks @tobie and @plinss!

vijaybh · 2016-10-06T16:41:57Z

index.bs

+|appId| [[FIDO-APPID]] to overwrite the otherwise computed |rpId|.
+
+: Extension identifier
+:: `webauthn_appid`


I wonder if fido_appid would be a more descriptive name here.

that would be fine with me, tho note that @rlin1 suggested webauthn_appid in #229 (comment) (rather than webauthn_u2f-appid -- i think fido_appid meets @rlin1 's goal (?). Also the section title is "FIDO AppId".

also: s/[RP]s/[RPS]/;

Also, "Authenticator" and "Client", in prose text should be lower case (except at beginning of sentence).

Done on renaming webauthn_appid to fido_appid

Done on correcting capitalization of Authenticator and Client

vijaybh · 2016-10-06T16:46:05Z

index.bs

+: Client processing
+:: The Client must perform AppId validation (as defined by [[FIDO-APPID]]).  If
+    valid, the value of |rpId| for all Client processing should be replaced by
+    the specified |appId|.


What if rpId is also specified in the options? Should this win, or should the call just error out? (FWIW I'm in favor of erroring out by throwing an exception - it is a programming error and the code should never be written that way.)

ok, i can agree with the approach Vijay suggests -- though the spec ought to explicitly indicate that rpId/appId is an "either/or", both in this extension section, and in the section on dictionary AssertionOptions.

Added language about it being an error to explicitly specify the rpId if using this extension.

Also added language to say that it is an error to use this extension outside the scope of getAssertion()

vijaybh · 2016-10-06T16:47:15Z

index.bs

+: Client argument
+:: A single UTF-8 encoded string specifying a FIDO |appId|.
+
+: Client processing


Stylistically, I wonder if this would be better written as "Replace step 3 in section foo with the following: ..." so this would be very clear. Would also help with defining error behavior as below.

Agreed, it would be good to be very explicit.

I have made this change as requested.

vijaybh · 2016-11-02T15:45:34Z

index.bs

+## FIDO AppId ## {#extension-appid}
+
+This authentication extension allows [RP]s who have previously registered an
+Authentication using the legacy FIDO JavaScript APIs to communicate with


"registered an Authentication" sounds weird. Maybe "registered a credential"?

Thanks for the suggestion. Wording improved.

vijaybh · 2016-11-02T15:49:41Z

index.bs

+    following procedure:  The client uses the value of |fido_appid| to perform
+    the AppId validation procedure (as defined by [[FIDO-APPID]]).  If valid,
+    the value of |rpId| for all client processing should be replaced by the
+    value of |fido_appid|.  It is an application error to both use this


Might be better to make the error explicit and move it to the beginning of the procedure. Something like "If {{AssertionOptions/rpId}} is present, reject promise with a DOMException whose name is "NotAllowedError", and terminate this algorithm. Otherwise, ..."

vijaybh · 2016-11-02T15:51:04Z

Added two editorial suggestions. Other than that this LGTM.

equalsJeffH · 2016-11-09T17:11:18Z

Looks fine to me.

vijaybh reviewed Oct 4, 2016

View reviewed changes

leshi force-pushed the acz branch from cb039c9 to 989f52e Compare October 5, 2016 04:00

Adding u2f appid extension

d5d88e4

Closes Issue #216

leshi force-pushed the acz branch from 989f52e to d5d88e4 Compare October 5, 2016 21:38

Adding updated references

103a5b7

vijaybh reviewed Oct 6, 2016

View reviewed changes

leshi added 2 commits November 1, 2016 21:49

addressing comments from issue peanut gallery :)

2ca56e5

typo fix

700775c

leshi modified the milestone: WD-03 Nov 2, 2016

vijaybh reviewed Nov 2, 2016

View reviewed changes

address vijay's comments

d1f5af9

leshi changed the title ~~Adding u2f appid extension~~ Adding fido appid extension Nov 2, 2016

vijaybh merged commit 2d6a7a5 into master Nov 9, 2016

jcjones deleted the acz branch November 9, 2016 18:14

vijaybh mentioned this pull request Nov 9, 2016

Moving callers from U2F to WebAuthn #216

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding fido appid extension #229

Adding fido appid extension #229

leshi commented Oct 3, 2016

rlin1 commented Oct 4, 2016

vijaybh Oct 4, 2016

leshi Oct 5, 2016 •

edited

Loading

tobie Oct 5, 2016 •

edited

Loading

leshi commented Oct 5, 2016

leshi commented Oct 5, 2016

tobie commented Oct 5, 2016 •

edited

Loading

plinss commented Oct 5, 2016

jcjones commented Oct 5, 2016 •

edited by leshi

Loading

leshi commented Oct 5, 2016

vijaybh Oct 6, 2016

equalsJeffH Oct 12, 2016 •

edited

Loading

equalsJeffH Oct 14, 2016 •

edited

Loading

leshi Nov 2, 2016

vijaybh Oct 6, 2016

equalsJeffH Oct 14, 2016

leshi Nov 2, 2016

vijaybh Oct 6, 2016

equalsJeffH Oct 14, 2016 •

edited

Loading

leshi Nov 2, 2016

vijaybh Nov 2, 2016

leshi Nov 2, 2016

vijaybh Nov 2, 2016

leshi Nov 2, 2016

vijaybh commented Nov 2, 2016

equalsJeffH commented Nov 9, 2016

Adding fido appid extension #229

Adding fido appid extension #229

Conversation

leshi commented Oct 3, 2016

rlin1 commented Oct 4, 2016

Choose a reason for hiding this comment

leshi Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

tobie Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

leshi commented Oct 5, 2016

leshi commented Oct 5, 2016

tobie commented Oct 5, 2016 • edited Loading

plinss commented Oct 5, 2016

jcjones commented Oct 5, 2016 • edited by leshi Loading

leshi commented Oct 5, 2016

Choose a reason for hiding this comment

equalsJeffH Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

equalsJeffH Oct 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH Oct 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vijaybh commented Nov 2, 2016

equalsJeffH commented Nov 9, 2016

leshi Oct 5, 2016 •

edited

Loading

tobie Oct 5, 2016 •

edited

Loading

tobie commented Oct 5, 2016 •

edited

Loading

jcjones commented Oct 5, 2016 •

edited by leshi

Loading

equalsJeffH Oct 12, 2016 •

edited

Loading

equalsJeffH Oct 14, 2016 •

edited

Loading

equalsJeffH Oct 14, 2016 •

edited

Loading